Extract model type dropdown to its own component#2833
Conversation
| input: argumentsList.length === 0 ? "Argument[this]" : "Argument[0]", | ||
| output: "ReturnValue", | ||
| kind: "value", | ||
| ...modeledMethod, |
There was a problem hiding this comment.
Note that this seems a bit off - we're spreading in the middle of the object creation, which means we could lose the property values we've set above. Seems like we should move the spread operation to L45 instead (or just do assignments explicitly).
For now I wanted to make minimal changes to the logic so I've left it as is.
There was a problem hiding this comment.
I agree, let's not change this in this PR, but I agree it looks odd to do the spread in the middle of the arguments. It would be good to investigate why it is this way, but overall I'd say lets just write it all out explicitly and not use the spread.
There was a problem hiding this comment.
I think we can just remove this spread operator. It seems like all fields are overridden, so this doesn't add any value. We can use an object spread operator on method instead, but this can be at the end of this object.
There was a problem hiding this comment.
Thanks all - I'll crate a separate PR for it.
This will allow us to reuse code for the new method modeling panel.
I've not added any unit tests/stories - we can do that in a separate issue.
Checklist
N/A:
ready-for-doc-reviewlabel there.